SentryGraphQLHttpFailedRequestHandler improved issue grouping. #4762
Conversation
ericsampson
requested review from
Flash0ver and
jamescrosswell
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| #endif | ||
| // Wrap in a new exception to preserve the stack trace when thrown and caught. | ||
| // The inner exception will have the full stack trace for better issue grouping. | ||
| throw new GraphQLHttpRequestException(errorMessage, exception); |
There was a problem hiding this comment.
Bug: Inner exception missing stack trace on pre-.NET 5
On pre-.NET 5 platforms, the inner exception won't have a stack trace because it's never thrown and SetCurrentStackTrace isn't available. The wrapper exception at line 45 only captures a minimal stack trace from the immediate throw/catch within the same method. To match the HTTP handler pattern, the inner exception should be thrown and caught on pre-.NET 5 platforms before wrapping, similar to the try-catch pattern used in SentryHttpFailedRequestHandler.
There was a problem hiding this comment.
I think that's fine.
There was a problem hiding this comment.
There's something weird about wrapping an exception that we just created (on line 36) in another identical exception.
Could we do this the same way as the other handler:
sentry-dotnet/src/Sentry/SentryHttpFailedRequestHandler.cs
Lines 40 to 54 in ab2d7c4
2 participants
Applies the same improvements for GraphQL as in this PR for plain HTTP: #4724